[Feat] CLI Login by HeshamHM28 · Pull Request #928 · codeflash-ai/codeflash

HeshamHM28 · 2025-11-17T22:24:23Z

Fixes cf-841

Screen.Recording.2025-11-18.at.12.05.33.AM.mov

PR Type

Enhancement

Description

Add OAuth PKCE login flow
CLI prompt supports login or API key
Save OAuth token as API key
Improve auth success/error UX

Diagram Walkthrough

flowchart LR
  CLI["CLI init: prompt_api_key"] -- "Choose Login" --> OAuth["perform_oauth_signin()"]
  CLI -- "Choose API key" --> Manual["enter_api_key_and_save_to_rc()"]
  OAuth -- "Start local server" --> Server["HTTP /callback + /status"]
  OAuth -- "Open browser" --> AuthPage["CodeFlash OAuth page"]
  AuthPage -- "Redirect with code/state" --> Server
  Server -- "Provide status" --> Browser["Loading/Success/Error UI"]
  OAuth -- "Exchange code for token" --> Token["API key (access_token)"]
  Token -- "Save to RC + env" --> CLI

File Walkthrough

Relevant files

Enhancement

cmd_init.py `CLI prompt adds OAuth login path` codeflash/cli_cmds/cmd_init.py Add OAuth signin option to API key prompt Integrate `perform_oauth_signin` and save token Preserve existing manual API key flow Telemetry events for both auth paths	+54/-3
oauth_handler.py `New OAuth handler with PKCE and local HTTP` codeflash/code_utils/oauth_handler.py Implement OAuth PKCE flow with local server Serve callback and status endpoints with HTML UX Exchange auth code for API token via cfwebapp Expose `perform_oauth_signin` helper for CLI	+744/-0

github-actions · 2025-11-17T22:25:19Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns Open redirect/callback exposure: The local server’s /status endpoint sets "Access-Control-Allow-Origin: *" which allows any web page to poll local auth status. While data is minimal, it could leak success state. Consider restricting CORS or removing the header.
⚡ Recommended focus areas for review Typo/UX The auth choice label says "Login in with Codeflash" which seems grammatically off; consider "Log in with CodeFlash" or "Sign in with CodeFlash" to avoid user confusion. auth_choices = [ "🔐 Login in with Codeflash", "🔑 Use Codeflash API key" ] questions = [ Port Binding Local HTTP server binds to "localhost" only; some systems/browsers may resolve to IPv6 (::1) causing callback issues. Consider binding to 127.0.0.1 explicitly and ensuring the redirect URI matches, or support dual-stack. handler_class = self.create_callback_handler() httpd = http.server.HTTPServer(("localhost", port), handler_class) def serve_forever_wrapper(): httpd.serve_forever() Error Messaging Token exchange sets a generic "Unauthorized" for most failures, discarding server-provided details. Preserve and surface specific error messages to improve debugging and UX, and reflect them via /status for the browser UI. except requests.exceptions.HTTPError as e: error_msg = f"HTTP {e.response.status_code}" try: error_data = e.response.json() error_msg = error_data.get("error_description", error_data.get("error", error_msg)) except: pass self.token_error = "Unauthorized" click.echo(f"❌ {self.token_error}") return None except Exception as e: self.token_error = "Unauthorized" click.echo(f"❌ {self.token_error}") return None

github-actions · 2025-11-17T22:25:37Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Bind port to localhost only Bind explicitly to localhost to avoid selecting an interface that's not loopback. This prevents the auth server from being unreachable when the browser is redirected to 127.0.0.1 while the server listens on 0.0.0.0 or another interface. It also reduces surface area by not exposing the callback server on all interfaces. codeflash/code_utils/oauth_handler.py [591-595] def get_free_port() -> int: - """Find an available port""" + """Find an available port bound to localhost""" with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: - s.bind(("", 0)) + s.bind(("127.0.0.1", 0)) return s.getsockname()[1] Suggestion importance[1-10]: 7 __ Why: Correctly narrows the bind to loopback which improves security and aligns with the redirect to localhost; a modest but meaningful improvement with low risk.	Medium
Possible issue	Use loopback and reuse address Use the same explicit loopback address consistently and enable socket reuse to avoid race conditions and "address already in use" errors on quick retries. This improves reliability of starting the local callback server across platforms. codeflash/code_utils/oauth_handler.py [608-620] def start_local_server(self, port: int) -> http.server.HTTPServer: """Start local HTTP server for OAuth callback""" handler_class = self.create_callback_handler() - httpd = http.server.HTTPServer(("localhost", port), handler_class) + http.server.HTTPServer.allow_reuse_address = True + httpd = http.server.HTTPServer(("127.0.0.1", port), handler_class) def serve_forever_wrapper(): httpd.serve_forever() - server_thread = threading.Thread(target=serve_forever_wrapper) + server_thread = threading.Thread(target=serve_forever_wrapper, name="cf-oauth-http") server_thread.daemon = True server_thread.start() return httpd Suggestion importance[1-10]: 6 __ Why: Consistently using 127.0.0.1 and enabling address reuse can reduce bind errors and exposure; helpful but not critical.	Low
General	Surface OAuth error details Include the actual OAuth error details in the CLI output to aid debugging and user remediation. Currently the message hides `oauth.error`, leaving users without actionable information. codeflash/code_utils/oauth_handler.py [721-724] if oauth.error: httpd.shutdown() - click.echo(f"❌ Authentication failed:") + click.echo(f"❌ Authentication failed: {oauth.error}") return None Suggestion importance[1-10]: 6 __ Why: Including `oauth.error` in the message improves diagnosability with minimal change and no downside, but is not critical to functionality.	Low

aseembits93 · 2025-11-17T22:25:52Z

screenshots if possible @HeshamHM28

misrasaurabh1 · 2025-11-17T23:12:16Z

+                return self._get_error_html("unauthorized", theme)
+
+            @staticmethod
+            def _get_loading_html(theme: str = "light") -> str:


why do we have html in the cli?

That is for the local server used on local machines.

misrasaurabh1 · 2025-11-17T23:14:05Z

+    # Setup PKCE
+    port = oauth.get_free_port()
+    redirect_uri = f"http://localhost:{port}/callback"
+    code_verifier, code_challenge = oauth.generate_pkce_pair()


is generating this in the cli side where the user can modify this generation logic a security vulnerability? what is user has full control of the generation string, would that degrade the security somehow?

It is safe because we ensure the user is authenticated in cf-webapp before exchanging the code. Even if the user overrides this value, they will still receive their own API key.

misrasaurabh1 · 2025-11-17T23:15:15Z

+    def start_local_server(self, port: int) -> http.server.HTTPServer:
+        """Start local HTTP server for OAuth callback."""
+        handler_class = self.create_callback_handler()
+        httpd = http.server.HTTPServer(("localhost", port), handler_class)


wait, we are doing local server way of authentication? this won't work in remote machines.
why are we not doing polling?

I implemented remote-machine logic in the last commit.

Co-authored-by: codeflash-ai[bot] <148906541+codeflash-ai[bot]@users.noreply.github.com>

codeflash-ai · 2025-11-18T22:10:17Z

This PR is now faster! 🚀 HeshamHM28 accepted my code suggestion above.

…n open the browser.

codeflash-ai · 2025-11-20T19:51:15Z

⚡️ Codeflash found optimizations for this PR

📄 11% (0.11x) speedup for `should_attempt_browser_launch` in `codeflash/code_utils/oauth_handler.py`

⏱️ Runtime : 248 microseconds → 224 microseconds (best of 63 runs)

A dependent PR with the suggested changes has been created. Please review:

⚡️ Speed up function should_attempt_browser_launch by 11% in PR #928 (feat/cli/login) #933

If you approve, it will be merged into this PR (branch feat/cli/login).

aseembits93

waiting for #931 to be merged @KRRT7 @HeshamHM28

misrasaurabh1 · 2025-11-21T19:21:37Z

why do we want TUI to be merged for this? we should merge this first

HeshamHM28 · 2025-11-21T19:28:07Z

why do we want TUI to be merged for this? we should merge this first

Because this change will require additional updates to TUI, so @KRRT7 suggested merging it after TUI.

misrasaurabh1 · 2025-11-21T19:29:15Z

merge this first for the CLI init. we still need to experiment more with TUI to make a decision to see if it is going to be better than the cli init for the users.

KRRT7 · 2025-11-21T19:29:29Z

let's merge it in now, the TUI flow currently requires a bit more testing before merging it in

HeshamHM28 added 3 commits November 14, 2025 21:10

POC

93990f5

add login flow

6082650

Enhance ui/ux

8d7bc7f

HeshamHM28 requested review from Saga4, mashraf-222, misrasaurabh1 and mohammedahmed18 November 17, 2025 22:24

github-actions Bot changed the title ~~[Feat] CLI Login~~ [Feat] CLI Login Nov 17, 2025

github-actions Bot added the Review effort 3/5 label Nov 17, 2025

aseembits93 and others added 2 commits November 17, 2025 14:38

Merge branch 'main' into feat/cli/login

9c670db

Fix formatting

0fcd102

misrasaurabh1 reviewed Nov 17, 2025

View reviewed changes

misrasaurabh1 requested changes Nov 17, 2025

View reviewed changes

HeshamHM28 and others added 3 commits November 18, 2025 16:46

change client id

76089d2

Add remote-machine logic.

6d1b3b8

Merge branch 'main' into feat/cli/login

be28362

HeshamHM28 requested a review from misrasaurabh1 November 18, 2025 21:52

codeflash-ai Bot reviewed Nov 18, 2025

View reviewed changes

Comment thread codeflash/code_utils/oauth_handler.py Outdated

Update codeflash/code_utils/oauth_handler.py

b4e87a0

Co-authored-by: codeflash-ai[bot] <148906541+codeflash-ai[bot]@users.noreply.github.com>

HeshamHM28 added 3 commits November 19, 2025 01:24

Change the logic related to the remote machine.

ebaa5ce

check for gui browser

3f49a50

Enhance the logic by adding a better function to detect whether we ca…

f58e685

…n open the browser.

codeflash-ai Bot mentioned this pull request Nov 20, 2025

⚡️ Speed up function should_attempt_browser_launch by 11% in PR #928 (feat/cli/login) #933

Closed

add new line

c465175

HeshamHM28 requested a review from aseembits93 November 20, 2025 21:03

Merge branch 'main' into feat/cli/login

a6d952f

aseembits93 approved these changes Nov 20, 2025

View reviewed changes

aseembits93 requested changes Nov 20, 2025

View reviewed changes

Merge branch 'main' into feat/cli/login

74098c9

aseembits93 approved these changes Nov 21, 2025

View reviewed changes

aseembits93 enabled auto-merge November 21, 2025 19:30

Merge branch 'main' into feat/cli/login

9cbc682

aseembits93 approved these changes Nov 21, 2025

View reviewed changes

aseembits93 self-requested a review November 21, 2025 20:31

aseembits93 approved these changes Nov 21, 2025

View reviewed changes

Merge branch 'main' into feat/cli/login

147fb31

Saga4 disabled auto-merge November 21, 2025 21:18

Saga4 merged commit fd2d2b8 into main Nov 21, 2025
21 checks passed

Saga4 deleted the feat/cli/login branch November 21, 2025 21:18

Conversation

HeshamHM28 commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

github-actions Bot commented Nov 17, 2025

PR Reviewer Guide 🔍

Uh oh!

github-actions Bot commented Nov 17, 2025

PR Code Suggestions ✨

Uh oh!

aseembits93 commented Nov 17, 2025

Uh oh!

misrasaurabh1 Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

HeshamHM28 Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

misrasaurabh1 Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

HeshamHM28 Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

misrasaurabh1 Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

HeshamHM28 Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codeflash-ai Bot commented Nov 18, 2025

Uh oh!

codeflash-ai Bot commented Nov 20, 2025

⚡️ Codeflash found optimizations for this PR

📄 11% (0.11x) speedup for should_attempt_browser_launch in codeflash/code_utils/oauth_handler.py

A dependent PR with the suggested changes has been created. Please review:

⚡️ Speed up function should_attempt_browser_launch by 11% in PR #928 (feat/cli/login) #933

Uh oh!

aseembits93 left a comment

Choose a reason for hiding this comment

Uh oh!

misrasaurabh1 commented Nov 21, 2025

Uh oh!

HeshamHM28 commented Nov 21, 2025

Uh oh!

misrasaurabh1 commented Nov 21, 2025

Uh oh!

KRRT7 commented Nov 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

HeshamHM28 commented Nov 17, 2025 •

edited

Loading

📄 11% (0.11x) speedup for `should_attempt_browser_launch` in `codeflash/code_utils/oauth_handler.py`

⚡️ Speed up function `should_attempt_browser_launch` by 11% in PR #928 (`feat/cli/login`) #933